Skip to content

Forms: disable device visibility on fields/inputs, honor per-viewport hiding on labels#49973

Merged
CGastrell merged 14 commits into
trunkfrom
update/forms-disable-field-visibility-support
Jul 2, 2026
Merged

Forms: disable device visibility on fields/inputs, honor per-viewport hiding on labels#49973
CGastrell merged 14 commits into
trunkfrom
update/forms-disable-field-visibility-support

Conversation

@CGastrell

@CGastrell CGastrell commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

Interim measure for FORMS-694 while we decide whether to fully support per-field device visibility.

What the GB control actually does. The block "visibility" support exposes two modes behind a single supports.visibility boolean: Hide everywhere (writes blockVisibility = false) and per-viewport "Hide on mobile/tablet/desktop" (writes { viewport: … }).

How they behave on our (dynamic) form blocks:

  • Per-viewport "Hide on…" is not honored — fields flatten into a shortcode-style array and bypass core's render_block class injection, so wp-block-hidden-* + the media-query CSS never reach the field. Worse, it can't be made safe on a required field: both server (validate()) and client (isFormValid) validation are viewport-blind, so a required field hidden on mobile stays required but unfillable → dead form.
  • "Hide everywhere" does work on fields (the field's shortcode is dropped before parse, so it's never validated/submitted — required-safe), but the single boolean bundles it with the broken per-viewport mode and we can't split them.
  • Inputs and choice/option blocks are inert in every mode (the field renderer reads their attrs directly and discards their rendered output).

So this disables visibility on every field, input, and choice/option block (the only one that keeps it is the label — see below).

Labels keep their support — and now honor per-viewport hiding too. Label hiding never touches input/validation/submission (it's pure presentation), so the viewport pitfall doesn't apply. Full-hide was already wired via labelhiddenbyblockvisibility; this adds the per-viewport case: the block→shortcode bridge reads blockVisibility.viewport and adds the same wp-block-hidden-{mobile,tablet,desktop} classes Gutenberg would to the label the field renderer emits. The matching media-query CSS is already registered by core's render_block visibility filter (the label keeps visibility support, so the filter runs on it — it just couldn't attach the class because that output is discarded). Applied across the default, outlined, animated, and below label styles.

Applied consistently on both registrations:

  • JS — shared/settings/index.js (all field blocks), plus per-block visibility: false on the input variants (input, input-range, input-rating, input-image-option, phone-input, dropzone) and choice/option blocks (option, options, fieldset-image-options, deprecated field-option-*).
  • PHP — a register_block_type_args filter matching all jetpack/field-*, jetpack/input*, phone-input, dropzone, and the option containers, mirroring the JS.

Note: this currently means a per-block opt-out duplicated across ~15 blocks. The cleaner end state — a shared supports array array_merge'd into each declaration, mirroring the JS defaultSettings — is tracked as a follow-up (TODO in the PHP filter).

Related

Does this pull request change what data or activity we track or use?

No.

Testing instructions

  • In the block editor, add a Form block and select any field, its input, or a choice/option — the visibility control is gone. Select a field's label — it's still there (the only forms block that keeps it). Select a non-form block (e.g. a paragraph) — unaffected.
  • Set a field's label to "Hide on mobile" and view the form on the frontend: the <label> gets wp-block-hidden-mobile and the @media (width <= 480px){…display:none} rule is emitted, so it hides on mobile. Full-hide on a label still removes it entirely.
  • Verified registration parity both sides: editor wp.blocks.hasBlockSupport('jetpack/field-name','visibility',true) === false for fields/input and true for labels/options; server WP_Block_Type_Registry reports the same. PHP Contact_Form_Block_Test passes.

Deferred (separate decision, FORMS-694)

Fully supporting per-field device visibility would require honoring viewport hiding inside the forms render pipeline and a viewport flag on the submission so required/validation can be skipped for the hidden viewport on both client and server — plus token handling. Likely not worth it; tracked in FORMS-694.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the update/forms-disable-field-visibility-support branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack update/forms-disable-field-visibility-support

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

@github-actions github-actions Bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jun 25, 2026
@CGastrell CGastrell added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jun 25, 2026
@CGastrell CGastrell self-assigned this Jun 25, 2026
@CGastrell CGastrell marked this pull request as ready for review June 25, 2026 14:23
@jp-launch-control

jp-launch-control Bot commented Jun 25, 2026

Copy link
Copy Markdown

Code Coverage Summary

Coverage changed in 3 files.

File Coverage Δ% Δ Uncovered
projects/packages/forms/src/contact-form/class-contact-form-field.php 1114/1819 (61.24%) -0.69% 22 💔
projects/packages/forms/src/blocks/contact-form/class-contact-form-block.php 618/682 (90.62%) 0.21% 0 💚
projects/packages/forms/src/contact-form/class-contact-form-plugin.php 705/1559 (45.22%) 0.18% 0 💚

Full summary · PHP report · JS report

If appropriate, add one of these labels to override the failing coverage check: Covered by non-unit tests Use to ignore the Code coverage requirement check when E2Es or other non-unit tests cover the code Coverage tests to be added later Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR I don't care about code coverage for this PR Use this label to ignore the check for insufficient code coveage.

Device/viewport visibility ("Hide on…") is not honored on the field wrapper
at render time — the field render pipeline bypasses core's render_block
visibility filter — so the control did nothing. Disable the 'visibility'
block support on field and input blocks on both the JS (shared/settings,
input) and PHP (register_block_type_args filter) registrations so the
non-functional option no longer appears. Labels keep visibility support.

See FORMS-694.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CGastrell CGastrell force-pushed the update/forms-disable-field-visibility-support branch from aa49d16 to 5d61ca2 Compare June 26, 2026 12:31
CGastrell and others added 3 commits June 26, 2026 10:15
The per-viewport "Hide on…" option is the part that isn't honored (and on a
required field can't be made safe); "hide everywhere" does work for fields,
but the GB control bundles both under one supports boolean, so we disable it
wholesale on fields/inputs as an interim. Comment/changelog wording only — no
functional change. Labels untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ktop)

Labels kept blockVisibility support but only full-hide was wired; the
per-viewport selection was ignored because the field renderer discards the
label's own rendered output (where Gutenberg would have added the
wp-block-hidden-* classes). Read blockVisibility.viewport in the block->shortcode
bridge and add the same wp-block-hidden-{mobile,tablet,desktop} classes to the
label the field renderer emits; the matching media-query CSS is already
registered by core's render_block visibility filter (label keeps support).
Also apply label classes to the 'below' style-variation label for parity.

Verified on the frontend: hidden-on-mobile label gets the class and the
@media (width <= 480px){.wp-block-hidden-mobile{display:none}} rule is emitted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CGastrell CGastrell changed the title Forms: disable the non-functional device visibility control on fields and inputs Forms: disable device visibility on fields/inputs, honor per-viewport hiding on labels Jun 29, 2026
@LiamSarsfield

Copy link
Copy Markdown
Contributor

Nice work on this, the interim approach makes sense. I double-checked the part everyone was unsure about: supports.visibility = false is the correct WP 7.0 opt-out key, and the label mapping reads the right metadata.blockVisibility.viewport.* shape, so the core of this is solid. A couple of things Claude/Codex spotted that may be worth addressing but will defer to you:

  • The control still shows on the other input blocks. The disable covers jetpack/input and jetpack/field-*, but a few siblings slip through: jetpack/phone-input, jetpack/input-rating, jetpack/input-range, and jetpack/input-image-option have no visibility: false in their JS and don't match the PHP filter. They flatten through the same shortcode path, so the dead "Hide on…" control is still reachable on the telephone/rating/slider/image-select inner inputs. Might be the perfect excuse to do the shared supports list your TODO already calls out?

  • Per-viewport label hide drops the input's accessible name. Hiding a label on a viewport adds wp-block-hidden-{viewport} (display:none), but the aria-label fallback only fires on full-hide, so the input ends up with no accessible name on that viewport. Happy either way: wire the fallback into the per-viewport path, or just note it's visual-only for now.

The standard jetpack/input was disabled, but the field-specific input blocks
(input-range/slider, input-rating, input-image-option, phone-input, dropzone/file)
define their own supports and don't share defaultSettings, so the control still
showed on them. Add visibility:false to each (JS) and broaden the PHP filter to
match all jetpack/input* plus phone-input and dropzone. Verified in-editor: every
field-* and input block now hides the control; only label keeps it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
LiamSarsfield
LiamSarsfield previously approved these changes Jun 29, 2026

@LiamSarsfield LiamSarsfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input-block gap from last round. phone-input, range/rating/image, and dropzone are covered now in both JS and PHP, and the visibility key checks out against WP 7.0 core. 🚀

Some nice to haves that may be worth addressing:

Tests. Only the jetpack/input supports line is locked down today, so reverting the per-viewport label mapping, the full-hide case, or any widened input would stay green. That's the gap that needed a follow-up commit this round. A @dataProvider test on disable_field_visibility_support() across the name matrix (a field, a field-option-*, the prefix inputs, phone-input/dropzone, a non-forms block), plus a couple of Contact_Form_Plugin_Test cases for blockVisibility = false and viewport.mobile = false, covers it.

Grouped fields skip full-hide on their label. render_legend_as_label() never checks labelhiddenbyblockvisibility, so a fully-hidden label still renders on radio, checkbox-multiple, image-select, and rating, while render_label() honors it. It predates this PR, but since this is the change that makes labels honor visibility, finishing it here (or a fast-follow) would round it out. The per-viewport classes already reach the legend, so only the full-hide branch is missing.

Per team decision, the choice/option blocks (option, options, fieldset-image-options,
and the deprecated field-option-radio/checkbox) are inputs in effect — they flatten
through the same field-shortcode path, so visibility is inert on them. Add
visibility:false (JS) and extend the PHP filter (drop the field-option-* exclusion;
add the option containers). Label is now the only forms block keeping the control.

Verified in-editor: every field/input/option block hides the control; only label shows it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CGastrell and others added 2 commits June 29, 2026 13:24
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y tests

- render_legend_as_label() now respects labelhiddenbyblockvisibility, so a
  fully-hidden label is dropped on radio/checkbox-multiple/image-select/rating
  fields too (it already honored per-viewport hide via label_classes). Mirrors
  render_label(); addresses Liam's review.
- Add a dataProvider matrix test for disable_field_visibility_support() (field,
  field-option, all input variants, option containers, label, non-forms block).
- Add label blockVisibility tests for block_attributes_to_shortcode_attributes:
  full-hide sets the flag, per-viewport adds wp-block-hidden-* classes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@LiamSarsfield LiamSarsfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this one. The disable approach is clean and the new test suites are a great addition.

I checked the WP 7.0 source since the feature isn't in-repo: supports.visibility = false is the correct opt-out and the per-viewport label CSS holds up, so the approach is sound. Three things worth a look:

  • Existing "hide everywhere" fields reappear on the frontend. On released WP (6.9–7.0.x) the visibility render filter checks support before the full-hide branch, so once visibility = false a field saved metadata.blockVisibility === false stops hiding. Core fixed this on trunk (Trac #65389) but it's unreleased. Preserve saved full-hide for fields at render, or note it as interim behavior.
  • Grouped-field legend full-hide drops the accessible name. render_legend_as_label() returns '' with no aria-label fallback on the <fieldset>, where the input path adds one at :945. Mirror it.
  • The render guards have no render-level test. Deleting the early-return keeps the suite green.

Minor: the label's supports.blockVisibility = true opt-in isn't a real key; supports.visibility = true is the explicit form.

…gend a11y

Addresses review on #49973 (FORMS-694).

- Fields: add a render_block filter (drop_field_hidden_everywhere) that drops a
  field whose metadata.blockVisibility === false before Contact_Form::parse()
  sees it. Disabling the visibility *support* stopped core's release-WP filter
  from honoring saved full-hide (the reorder fix, gutenberg#78780, is
  unreleased), so previously-hidden fields would reappear. Doing the drop
  ourselves is version-independent and required-safe: the field is fully removed
  from the form, so its required check never runs.
- Grouped fields: when the legend label is fully hidden, move the label onto the
  <fieldset> as an aria-label so radio/checkbox-multiple/image-select/rating keep
  an accessible name, mirroring the single-input fallback.
- Fix the label's support key (blockVisibility -> visibility; the old key was a
  no-op) and update the registration test.
- Tests: unit + do_blocks integration coverage for the field drop; render-level
  coverage for the legend aria-label across both fieldset code paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ding

Optional follow-up to the legend a11y fix (FORMS-694). The visibility control
now survives only on the label block, so per-viewport hiding hides the
<label>/<legend> (display:none on that viewport) while the input stays visible —
leaving the field with no accessible name on that viewport.

Route every aria-label fallback site (input, phone, textarea, select, grouped
legend) through a single predicate that treats the label as hidden on either
full-hide (labelhiddenbyblockvisibility) or per-viewport (a wp-block-hidden-*
class on the label), and prefer the label text as the fallback name so it
matches the visible label where the per-viewport label still shows (falling back
to the placeholder only when there is no label). The render_label /
render_legend_as_label early-returns stay full-hide-only, so per-viewport keeps
the label rendered rather than removing it.

This is intentionally a standalone commit: if we'd rather keep the PR minimal and
treat per-viewport label hiding as visual-only for now, it can be dropped without
touching the rest of the fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CGastrell

Copy link
Copy Markdown
Contributor Author

Thanks @LiamSarsfield — all three addressed in 7d0a66f, plus the minor:

  • Fields reappearing: you're right that relying on core's filter breaks on released WP (the reorder in gutenberg#78780 is unreleased). Added a render_block filter that drops a field with blockVisibility === false ourselves, before Contact_Form::parse() — version-independent, and required-safe since the field is fully removed rather than validated. Covered by a do_blocks() integration test.
  • Legend accessible name: fixed — the <fieldset> now gets an aria-label on full-hide, across both fieldset paths (radio/checkbox-multiple/image-select via $fieldset_id, rating via sprintf).
  • No render-level test: added; deleting either the early-return or the aria-label injection now fails the suite.
  • Minor: corrected blockVisibilityvisibility and updated the registration test.

Separately, 1687ffb extends the aria-label fallback to per-viewport label hiding too (the label is display:none on a viewport while the input stays visible). It's a standalone commit — it does shift the fallback name from placeholder-first to label-first for consistency. Happy to drop it if we'd rather treat per-viewport as visual-only for now; wanted your read.

LiamSarsfield
LiamSarsfield previously approved these changes Jul 2, 2026

@LiamSarsfield LiamSarsfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The disable-plus-drop approach is clean, and the render tests are a good addition. I ran the branch on a test site and opened an existing form whose field had a saved visibility setting; it loads clean, with no invalid-content warning now the support is gone. Happy to approve.

Some minor optional nits Claude found

  • The slider field misses the hidden-label aria-label fallback. render_slider_field() renders the label, then a bare <input type="range">, so a slider whose label is hidden (hide-everywhere or a per-viewport toggle) has no accessible name. Every other single input got the fallback this round; the slider got skipped. Same call as the others (is_label_hidden_by_block_visibility() + get_block_visibility_aria_label()), plus a hidden-label slider test.

  • While you're in there: the fallback receives the pre-built placeholder='…' fragment instead of the raw value, so an empty-label field can render aria-label="placeholder='…'", and the phone country selector can emit an empty aria-label. Pass the raw placeholder through, and skip the attribute when both label and placeholder are empty. Good moment to lift the input aria-label logic into a small helper like the legend one so every path shares it.

  • Small one: the changelog entry starts with Forms: inside the forms package, which the guidance says to drop.

  • Minor consistency: label/index.js still sets blockVisibility: true while the PHP now uses visibility. Works today since visibility defaults on, but the keys should match.

…S key, changelog

Follow-up to the review on #49973 (FORMS-694):

- Slider: the bare <input type="range"> was the one single input missing the
  hidden-label aria-label fallback; add it.
- Lift the input aria-label logic into a shared get_hidden_label_aria_label_attr()
  helper used by every path (input, phone, textarea, select, slider, legend) so
  none get skipped again. The fallback now takes the raw placeholder (not the
  pre-built placeholder='…' fragment), so an empty-label field no longer renders
  aria-label="placeholder='…'", and the attribute is omitted entirely when there
  is no label and no placeholder (no empty aria-label).
- label/index.js: match the PHP fix — blockVisibility -> visibility support key.
- Changelog: drop the redundant "Forms:" prefix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CGastrell

Copy link
Copy Markdown
Contributor Author

Thanks — all four fixed in 6250c4f:

  • Slider: added the missing aria-label fallback (+ test).
  • Placeholder fragment: good catch — render_field passes the built placeholder='…' fragment down. Lifted every aria-label path (input, phone, textarea, select, slider, legend) into one shared helper that uses the raw placeholder and drops the attribute when there's no name, so no empty/fragment aria-label and nothing to skip next time.
  • label/index.js: blockVisibilityvisibility.
  • Changelog: dropped the Forms: prefix.

LiamSarsfield
LiamSarsfield previously approved these changes Jul 2, 2026

@LiamSarsfield LiamSarsfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
One non-blocking follow-up worth logging: a file upload field whose label is hidden falls back to the generic aria-label="Select a file to upload.", so a screen reader announces two hidden-label upload fields the same way. Same fix as the slider: wire the shared helper into the dropzone button, or point aria-labelledby at the label.
Happy to see it in a separate PR though

…its label is hidden

Follow-up to the review on #49973 (FORMS-694). The file field's dropzone
<div role="button"> is the interactive control, but its accessible name is
otherwise the generic "Select a file to upload.", so two upload fields whose
labels are hidden are announced identically.

Prefix the field name onto the dropzone's aria-label when the label is hidden
(full or per-viewport), reusing the shared block-visibility name resolver — the
same fallback the other single inputs got. Extracted into
get_file_dropzone_aria_label() so it's unit-testable (the full file render
short-circuits without an active Jetpack).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CGastrell

Copy link
Copy Markdown
Contributor Author

Thanks — done in 3dff8d4. Went ahead and included it here since it's the same theme and the helper was already in place.

You're right that the dropzone <div role="button"> is the field's real control (the label's for points at the container div, so it's cosmetic). It now prefixes the field name onto the dropzone's aria-label when the label is hidden — "{label}: Select a file to upload." — via the same shared resolver, so two hidden-label uploads read distinctly. Pulled it into get_file_dropzone_aria_label() for a unit test since the full file render needs an active Jetpack.

… paths

- ReflectionMethod::setAccessible() is deprecated on PHP 8.5 (a no-op since
  8.1), and the forms suite fails the build on deprecations, so the file
  dropzone test broke CI on 8.5. Guard the call with PHP_VERSION_ID < 80100,
  matching the existing pattern in Contact_Form_Block_Test.
- Add render tests exercising the hidden-label aria-label fallback through the
  textarea, select and country-selector phone paths (the remaining changed
  lines that weren't covered), which also verifies the fix across those types.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@LiamSarsfield LiamSarsfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The label-first aria-label fallback and the dropzone naming are both spot on, and dropping hide-everywhere fields before parse is the right way to keep required fields safe. Thanks for the quick turnaround on this

@CGastrell CGastrell merged commit 9aeb2f8 into trunk Jul 2, 2026
76 of 77 checks passed
@CGastrell CGastrell deleted the update/forms-disable-field-visibility-support branch July 2, 2026 17:21
@github-actions github-actions Bot added [Status] UI Changes Add this to PRs that change the UI so documentation can be updated. and removed [Status] In Progress [Status] Needs Review This PR is ready for review. labels Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants